[Local Catalog] Incremental sync: persist incremental sync date in Site storage entity #16108

jaclync · 2025-09-09T06:13:10Z

Based on [Local Catalog] Incremental sync: persist incrementally changed products/variations to storage #16106
⚠️ Please review after [Local Catalog] Incremental sync: persist incrementally changed products/variations to storage #16106 is approved

Summary

Add posLastIncrementalSyncDate column to Site table to track last incremental sync date for POS
Create POSSite model with conversion utilities to bridge Storage and Yosemite layers
Add loadSite and updateSite methods to POSCatalogPersistenceService for site management
Update POSCatalogIncrementalSyncService to use persisted sync date when determining modifiedAfter parameter
Add test coverage for site persistence and incremental sync behavior

Steps to reproduce

Just CI is sufficient, as the incremental service isn't integrated with the app yet.

Testing information

I tested that incremental sync worked after a full sync for a big site, making changes to at least one product and variation while full sync is in progress:

Task {
   do {
        let date = Date()
        let catalog = try await syncService.startFullSync(for: siteID)
        print("Full sync completed with \(catalog.products.count) products and \(catalog.variations.count) variations for siteID \(siteID)")

        try await incrementalSyncService.startIncrementalSync(for: siteID, lastFullSyncDate: date)
        print("Incremental sync completed")
    } catch {
        print("Full sync failed for siteID \(siteID) with error: \(error)")
    }
}

After all syncs were complete, I verified that posLastIncrementalSyncDate was set to the correct datetime in the site table in the sqlite database.

I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

…for POS usage.

…atalogIncrementalSyncService`.

wpmobilebot · 2025-09-09T06:22:52Z

📲 You can test the changes from this Pull Request in WooCommerce iOS Prototype by scanning the QR code below to install the corresponding build.

	App Name	WooCommerce iOS Prototype
	Build Number	`pr16108-f625bab`
	Version	`23.2`
	Bundle ID	`com.automattic.alpha.woocommerce`
	Commit	`f625bab`
	Installation URL	3ue8ostq494jg

Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

joshheald

We still need to check how this works a little more with site credential login.

Here's what I think happens:

Log in with one site's credentials and sync – everything gets persisted to the -1 site ID
Logout – I don't think the site is deleted
Log in with a different site's credentials – incremental sync will happen, based on the date stored here.
All the wrong data will still be stored.

If we delete the site on logout, none of this is an issue... but I don't think that will happen.

I want to check what happens with the user defaults site-specific storage too.

joshheald · 2025-09-09T10:53:49Z

Modules/Sources/Storage/GRDB/Migrations/V001InitialSchema.swift

    static func createSiteTable(_ db: Database) throws {
        try db.create(table: "site") { siteTable in
            siteTable.primaryKey("id", .integer).notNull()
+            siteTable.column("posLastIncrementalSyncDate", .datetime)


I think we should avoid mentioning POS in the database layer, until we're sure it's just for POS.

If the store management part of the app starts using this database, we'll want to keep sync working properly. Realistically, I think that means having a single sync controller, not separate ones for POS and the main app. Especially for shared entities such as products/variations.

Makes sense, renamed posLastIncrementalSyncDate to lastCatalogIncrementalSyncDate in f625bab.

joshheald · 2025-09-09T10:57:28Z

Modules/Sources/Yosemite/Tools/POS/POSCatalogPersistenceService.swift

+            let persistedSite = PersistedSite(from: site)
+            try persistedSite.update(db)


Perhaps PersistedSite needs MutableFetchableRecord?

Seems a little odd to fetch the site and never use it, just replace it.

Seems a little odd to fetch the site and never use it, just replace it.

Did you mean fetching the site from the database? L167 is converting a POSSite to PersistedSite without going through the database.

These two lines are to update the Site in the database from a given POSSite. Feel free to share better ways to do update an existing record.

joshheald · 2025-09-09T10:57:53Z

Modules/Tests/YosemiteTests/Tools/POS/POSCatalogPersistenceServiceTests.swift

+    private var db: GRDBDatabaseConnection {
+        grdbManager.databaseConnection
+    }


jaclync · 2025-09-10T08:13:54Z

If we delete the site on logout, none of this is an issue... but I don't think that will happen.

Sounds like a similar discussion point to the thread in #16100 (comment). Let's continue the discussion there.

I want to check what happens with the user defaults site-specific storage too.

On logout (AuthenticatedState.willLeave), a reset action is dispatched to reset the store settings which deletes the plist file:

woocommerce-ios/WooCommerce/Classes/Yosemite/AuthenticatedState.swift

Line 233 in 3914fc5

let resetGeneralStoreSettings = AppSettingsAction.resetGeneralStoreSettings

jaclync · 2025-09-11T00:55:50Z

Merging now to make dependent work easier (integrating the incremental sync service to sync coordinator). Feel free to leave suggestions and I'll respond later.

jaclync added 3 commits September 9, 2025 13:41

Add posLastIncrementalSyncDate: Date? column to Site table.

83c0665

Add POSSite to Yosemite that can be converted from PersistedSite …

a86d762

…for POS usage.

Persist last incremental sync date in Site and replace usage in `POSC…

cdb08d6

…atalogIncrementalSyncService`.

jaclync changed the base branch from trunk to feat/WOOMOB-1298-incremental-sync-persistence September 9, 2025 06:13

jaclync added type: task An internally driven task. feature: POS labels Sep 9, 2025

jaclync added this to the 23.3 milestone Sep 9, 2025

jaclync changed the title ~~Persist incremental sync date in Site for POS catalog sync~~ [Local Catalog] Incremental sync: persist incremental sync date in Site storage entity Sep 9, 2025

Ignore periphery for POSSite file.

331b152

jaclync changed the title ~~[Local Catalog] Incremental sync: persist incremental sync date in Site storage entity~~ [Local Catalog] Incremental sync: persist incremental sync date in Site table Sep 9, 2025

jaclync requested a review from joshheald September 9, 2025 06:53

jaclync changed the title ~~[Local Catalog] Incremental sync: persist incremental sync date in Site table~~ [Local Catalog] Incremental sync: persist incremental sync date in Site storage entity Sep 9, 2025

jaclync mentioned this pull request Sep 9, 2025

[POS][Local Catalog] Always full sync, if site not in database #16099

Merged

1 task

joshheald approved these changes Sep 9, 2025

View reviewed changes

Base automatically changed from feat/WOOMOB-1298-incremental-sync-persistence to trunk September 10, 2025 06:28

jaclync added 2 commits September 10, 2025 14:50

Merge branch 'trunk' into feat/WOOMOB-1289-persist-incremental-sync-date

438935f

Rename posLastIncrementalSyncDate to lastCatalogIncrementalSyncDate.

f625bab

joshheald mentioned this pull request Sep 10, 2025

[Woo POS][Local Catalog] Store full sync date on site entity #16115

Merged

1 task

jaclync merged commit 801d7c9 into trunk Sep 11, 2025
14 checks passed

jaclync deleted the feat/WOOMOB-1289-persist-incremental-sync-date branch September 11, 2025 00:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[Local Catalog] Incremental sync: persist incremental sync date in Site storage entity #16108

[Local Catalog] Incremental sync: persist incremental sync date in Site storage entity #16108

jaclync commented Sep 9, 2025 •

edited

Loading

Uh oh!

wpmobilebot commented Sep 9, 2025 •

edited

Loading

Uh oh!

joshheald left a comment

Uh oh!

joshheald Sep 9, 2025

Uh oh!

jaclync Sep 10, 2025

Uh oh!

joshheald Sep 9, 2025

Uh oh!

jaclync Sep 10, 2025

Uh oh!

joshheald Sep 9, 2025

Uh oh!

jaclync commented Sep 10, 2025 •

edited

Loading

Uh oh!

jaclync commented Sep 11, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		let persistedSite = PersistedSite(from: site)
		try persistedSite.update(db)

[Local Catalog] Incremental sync: persist incremental sync date in Site storage entity #16108

[Local Catalog] Incremental sync: persist incremental sync date in Site storage entity #16108

Conversation

jaclync commented Sep 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Steps to reproduce

Testing information

Uh oh!

wpmobilebot commented Sep 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

joshheald left a comment

Choose a reason for hiding this comment

Uh oh!

joshheald Sep 9, 2025

Choose a reason for hiding this comment

Uh oh!

jaclync Sep 10, 2025

Choose a reason for hiding this comment

Uh oh!

joshheald Sep 9, 2025

Choose a reason for hiding this comment

Uh oh!

jaclync Sep 10, 2025

Choose a reason for hiding this comment

Uh oh!

joshheald Sep 9, 2025

Choose a reason for hiding this comment

Uh oh!

jaclync commented Sep 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jaclync commented Sep 11, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jaclync commented Sep 9, 2025 •

edited

Loading

wpmobilebot commented Sep 9, 2025 •

edited

Loading

jaclync commented Sep 10, 2025 •

edited

Loading